Skip to content

fix(transformer): do not add __self when JSX is top-level#3281

Closed
overlookmotel wants to merge 1 commit intomainfrom
05-14-fix_transformer_do_not_add___self_when_jsx_is_top-level
Closed

fix(transformer): do not add __self when JSX is top-level#3281
overlookmotel wants to merge 1 commit intomainfrom
05-14-fix_transformer_do_not_add___self_when_jsx_is_top-level

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented May 14, 2024

Fixes a couple of small problems with #3258.

Now does not add __self in a couple of places where there is no this.

  1. Top level
const x = <Foo>blah</Foo>;
  1. In TS Module Block
declare namespace Foo {
  let x = <Bar>monkey</Bar>;
}

@graphite-app
Copy link
Contributor

graphite-app bot commented May 14, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @overlookmotel and the rest of your teammates on Graphite Graphite

@github-actions github-actions bot added the A-transformer Area - Transformer / Transpiler label May 14, 2024
@overlookmotel overlookmotel marked this pull request as ready for review May 14, 2024 17:25
@overlookmotel overlookmotel requested a review from Dunqing May 14, 2024 17:26
@codspeed-hq
Copy link

codspeed-hq bot commented May 14, 2024

CodSpeed Performance Report

Merging #3281 will not alter performance

Comparing 05-14-fix_transformer_do_not_add___self_when_jsx_is_top-level (c8de1d2) with main (482dcc0)

Summary

✅ 27 untouched benchmarks

@overlookmotel
Copy link
Member Author

I figured out what a TS Module Block is! 😆

@overlookmotel
Copy link
Member Author

Or maybe not... conformance is failing. I'll look into it.

@overlookmotel overlookmotel marked this pull request as draft May 14, 2024 17:27
@overlookmotel overlookmotel removed the request for review from Dunqing May 14, 2024 17:27
@overlookmotel
Copy link
Member Author

Well it seems I was wrong and adding __self in these cases is the correct behavior. I guess this at top level will refer to window in browser scripts, and undefined in ESM.

@Dunqing Well my review of #3258 wasn't my finest hour! I seem to have repeatedly got things wrong over and over again. Sorry for all the faffing around and time-wasting.

@Boshen Boshen deleted the 05-14-fix_transformer_do_not_add___self_when_jsx_is_top-level branch September 11, 2024 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-transformer Area - Transformer / Transpiler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant